Fix logging middleware body reading and register extra params globally#53
Merged
Fix logging middleware body reading and register extra params globally#53
Conversation
The logging middleware was calling response.json() and falling back to response.text() on the same cloned response. This fails when json() fails to parse (e.g., HTML error pages from Cloudflare) because json() consumes the body stream even when parsing fails. Fix: Read as text() first, then try JSON.parse() on the string. This only consumes the body once and handles non-JSON responses correctly.
The extra params (--extra-query, --extra-body, --extra-headers) were being parsed but never applied to requests. This fix registers the extra params middleware in the global middleware registry during BaseCommand.init(), so it applies to ALL HTTP clients (WebDAV, OCAPI, SLAS, ODS, MRT, Custom APIs).
clavery
changed the title
- Skip adding body to GET/HEAD requests which don't allow request bodies - Clean up global middleware registry between tests to prevent leakage - Fix query params to use modifiedRequest instead of original request
Collaborator
Author
|
skipping review to get e2e tests working. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes for HTTP middleware:
Fix body reading error - Fixes "Body is unusable: Body has already been read" error when API returns non-JSON error responses (e.g., Cloudflare 403 HTML pages)
Register extra params globally - Fixes
SFCC_EXTRA_HEADERS,SFCC_EXTRA_QUERY, andSFCC_EXTRA_BODYnot being applied to requests. The flags were parsed but never registered as middleware.Fix 1: Body Reading
The logging middleware was calling
response.json()and falling back toresponse.text()on the same cloned response. The Fetch API body can only be read once - even whenjson()fails to parse, it still consumes the stream.Solution: Read as
text()first, then tryJSON.parse()on the string.Fix 2: Extra Params Registration
The
--extra-query,--extra-body, and--extra-headersflags (and theirSFCC_*env vars) were parsed bygetExtraParams()but never applied to HTTP clients.Solution: Register the extra params middleware in
globalMiddlewareRegistryduringBaseCommand.init(). All HTTP clients (WebDAV, OCAPI, SLAS, ODS, MRT, Custom APIs) already check this registry.Test plan
SFCC_EXTRA_HEADERSappear in request logs